-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
config: support custom configuration of diff #116
base: main
Are you sure you want to change the base?
Conversation
c028c64
to
a570f5f
Compare
Would be really cool if the config could also be set via environments so no need to recompile. |
Good point, I can see how having something similar to the I can think of two kinds of API here:
I think the second option where we have one key that we parse if nicer. I wouldn't want to pull in an external crate for this, but simple key-value parsing is trivial. Would that work for your use case? |
Yeah, that's a good one. Also I would expect the following behavior:
|
I think the suggested syntax is very problematic: assert_eq!(config = config, 42, 43); When reading this code I'd assume that it asserts that To be honest I really don't see the point of overloading the usage of the macros that mimic the |
That's very valid. I do like how having a drop-in replacement eases transition burden from the standard macros, but you're right that there's no reason to make that the only API. Some sort of builder style API seems reasonable in that case: we already have a non-macro interface that's not publicly exposed. |
Right ... though it's not just the easy transition from std to pretty_assertions that's nice. I also appreciate how you can switch back and forth between std, pretty_assertions and even other crates like similar-asserts by just changing the imports. When it comes to code I think clarity is one of the most important goals, and from that standpoint I think that any overloading of the |
Supersedes #79
Relates to #38, #105, #114
This PR proposes an extensible system for customising diff output, by prefixing invocation with a
config = ...
argument.It's inspired by the tracing macros, which deal with the
log::log!
macros not being extensible with further suffix arguments, by adding prefix keyword arguments instead.This is the best design I've come across for extending standard library macros that take an arbitrary number of positional arguments - the
Config
struct itself can be extended in future by adding new builder methods.